Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automate configuration #189

Merged
merged 23 commits into from
Dec 24, 2024
Merged

Automate configuration #189

merged 23 commits into from
Dec 24, 2024

Conversation

xop5
Copy link
Collaborator

@xop5 xop5 commented Dec 19, 2024

No description provided.

@xop5 xop5 requested a review from ryanraaschCDC December 19, 2024 06:09
t.append(task_id)
return t
tasks.append(task_id)
return tasks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this add_task() function shouldn't be changed. It's currently setup to return the string task_id name if it's just a single task that gets added and a list if it is multiple. Returning a string is necessary for the new automation functions

Copy link
Contributor

@ryanraaschCDC ryanraaschCDC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing I found was changing the return of the add_task_to_job function. Other than that it looks good if you've tested out the changes you made to clients

@xop5 xop5 force-pushed the automate_configuration branch from c300159 to 8515a82 Compare December 23, 2024 15:35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this batch section was previously deleted but now it's back

return task_id

tasks.append(task_id)
return tasks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is still returning a list. We should just return task_id from this function and there's no need to append it to tasks. Check out the master branch for how it should work

@ryanraaschCDC
Copy link
Contributor

please run pre-commit on the files before merging

@xop5 xop5 merged commit af859fe into master Dec 24, 2024
4 checks passed
@xop5 xop5 deleted the automate_configuration branch December 24, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants